Skip to content

Conversation

@jonkoops
Copy link
Collaborator

@jonkoops jonkoops commented Oct 8, 2024

What kind of change does this PR introduce?

  • Bug Fix
  • Feature
  • Refactoring
  • Style
  • Build
  • Chore
  • Documentation
  • CI

Did you add tests for your changes?

  • Yes, my code is well tested
  • Not relevant

If relevant, did you update the documentation?

  • Yes, I've updated the documentation
  • Not relevant

Summary
Replaces the legacy version of Yarn with regular NPM. This removes a now unsupported version of Yarn and makes it easier for folks looking to make a contribution since NPM will be installed on their system alongside with Node.js.

I considered using the new version of Yarn, but it would require users to enable Corepack, which is slated to be removed from Node.js in the next major. Using NPM simplifies things for users wishing to contribute.

Explain the motivation for making this change. What existing problem does the pull request solve?
Try to link to an open issue for more information.

Does this PR introduce a breaking change?
No.

Other information
None.

@jonkoops jonkoops force-pushed the replace-legacy-yarn branch from 289d0be to efa5584 Compare October 8, 2024 10:35
@jonkoops
Copy link
Collaborator Author

jonkoops commented Oct 8, 2024

Looks like some older versions of Node.js don't like the new lockfile format, I will add some conditions there and mark this PR as a draft in the meantime.

@jonkoops jonkoops marked this pull request as draft October 8, 2024 10:43
@jonkoops jonkoops force-pushed the replace-legacy-yarn branch 2 times, most recently from e753732 to c0471d4 Compare October 8, 2024 10:50
@jonkoops jonkoops force-pushed the replace-legacy-yarn branch from c0471d4 to 864e666 Compare October 8, 2024 10:51
@jonkoops jonkoops marked this pull request as ready for review October 8, 2024 10:51
@jonkoops
Copy link
Collaborator Author

jonkoops commented Oct 8, 2024

Managed to fix the issue by installing a more recent versions of NPM that is compatible with Node.js 14 conditionally. @rolandjitsu WDYT?

@coveralls
Copy link

Pull Request Test Coverage Report for Build 9046629b0188dfb48f797d03c30cd9ce8aab4984-PR-89

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 92d2f2efaf6bed6f2ee7c860eeb9d6453ebabea3: 0.0%
Covered Lines: 80
Relevant Lines: 80

💛 - Coveralls

@coveralls
Copy link

Pull Request Test Coverage Report for Build 293aadd14759997a25f0a1de72d90200ee603e01-PR-89

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 92d2f2efaf6bed6f2ee7c860eeb9d6453ebabea3: 0.0%
Covered Lines: 80
Relevant Lines: 80

💛 - Coveralls

@rolandjitsu
Copy link
Collaborator

@jonkoops I still use yarn 😄 I don't think it's deprecated, it's a choice users can make via corepack (on latest node, a corepack enable will allow users to use yarn - on first use it installs the required version).

And we can also ensure that users use a specific version of yarn by adding a packageManager property in package.json.

@jonkoops
Copy link
Collaborator Author

jonkoops commented Oct 8, 2024

The only actively maintained version of Yarn is v4, the legacy version used here is v1, which has not been maintained since 2017. If you'd like I can upgrade the project with Yarn v4 instead, but it will come with some caveats:

  • Users will have to enable corepack increasing the barrier to entry over NPM
  • Corepack will be removed from Node.js in version 23, requiring an additional installation step.

Let me know what you'd like to do.

@rolandjitsu
Copy link
Collaborator

rolandjitsu commented Oct 8, 2024

@jonkoops I think it makes sense to transition back to npm in that case. So your changes are relevant.

@jonkoops
Copy link
Collaborator Author

jonkoops commented Oct 8, 2024

Can you merge this one in @rolandjitsu, or are there some changes you'd like to add? I have some other work ready to upgrade the GitHub Actions to the latest version that depends on this PR :)

@rolandjitsu rolandjitsu merged commit a1e91dc into react-dropzone:master Oct 8, 2024
6 checks passed
@jonkoops jonkoops deleted the replace-legacy-yarn branch October 8, 2024 21:54
@github-actions
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants